Skip to content

Conversation

@smohiudd
Copy link

Description

When path prefixes are used in FastAPI, the next and prev collection items links don't include the path prefix. See NASA-IMPACT/veda-backend#444 for a specific example. This PR uses self.resolve similar to other links (which include the path prefix).

method = self.request.method
if method == "GET":
href = merge_params(self.url, {"token": f"next:{self.next}"})
url = self.resolve(f"collections/{self.collection_id}/items")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😬 it seems that PagingLinks class is not only used in the /items endpoint but also in the all_collections method

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh I missed that. I guess that change is not going to work then

@vincentsarago
Copy link
Member

@smohiudd thanks for the PR

First we should add a test showing the wrong behaviour that this PR should fix
Second please use pre-commit to lint the code
Third (sorry), sadly I don't think we can hard code the /items path in the PagingLinks class, because this class will also be used for the collection_search (but it won't once https://github.com/stac-utils/stac-fastapi-pgstac/pull/155/files#diff-d46e0e1e3b57fcae2dd375220c7f5d0d4bbce77a459a02212b4b8d7c3857179f get resolved)

@smohiudd smohiudd closed this Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants